Skip to content

Use FFI/JNI for captureEnvelope on iOS and Android #3115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 33 commits into
base: deps/bump-ffigen
Choose a base branch
from

Conversation

buenaflor
Copy link
Contributor

@buenaflor buenaflor commented Jul 29, 2025

Step towards #1444

💚 How did you test it?

It's currently hard to mock JNI/FFI calls so we'll be relying on integration tests and verifying that method channel calls are not made anymore for captureEnvelope

Performance improvement is marginal but an improvement nonetheless


iOS:
FFI calls (26 samples):
Average ≈ 2 967.4 microseconds

Channel calls (25 samples):
Average ≈ 3 264.5 microseconds


Android:
JNI calls (33 samples):
Average ≈ 1 012.8 microseconds

Channel calls (32 samples):
Average ≈ 1 462.5 microseconds


📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

🔮 Next steps

@sentry review

Copy link

codecov bot commented Jul 29, 2025

Codecov Report

❌ Patch coverage is 58.82353% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.61%. Comparing base (1d6c314) to head (303cf57).

Files with missing lines Patch % Lines
...lutter/lib/src/native/java/sentry_native_java.dart 50.00% 4 Missing ⚠️
...tter/lib/src/native/cocoa/sentry_native_cocoa.dart 66.66% 3 Missing ⚠️
Additional details and impacted files
@@                 Coverage Diff                  @@
##           deps/bump-ffigen    #3115      +/-   ##
====================================================
- Coverage             89.85%   89.61%   -0.25%     
====================================================
  Files                    96       96              
  Lines                  3431     3448      +17     
====================================================
+ Hits                   3083     3090       +7     
- Misses                  348      358      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

github-actions bot commented Jul 29, 2025

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1258.76 ms 1274.46 ms 15.70 ms
Size 20.71 MiB 22.45 MiB 1.74 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
2cb90b9 1272.90 ms 1285.55 ms 12.66 ms
dbd526b 1244.78 ms 1259.02 ms 14.24 ms
81f83eb 1259.53 ms 1273.39 ms 13.86 ms
73dca78 1246.65 ms 1265.42 ms 18.76 ms
6b69699 1254.80 ms 1273.31 ms 18.52 ms
f761369 1261.69 ms 1277.82 ms 16.12 ms
640ad0c 1241.04 ms 1253.96 ms 12.92 ms
7cfbbd6 1270.63 ms 1285.36 ms 14.72 ms
e45c0e1 1269.08 ms 1278.83 ms 9.75 ms
cc4e375 1253.06 ms 1263.81 ms 10.75 ms

App size

Revision Plain With Sentry Diff
2cb90b9 7.86 MiB 9.54 MiB 1.69 MiB
dbd526b 7.86 MiB 9.44 MiB 1.58 MiB
81f83eb 7.86 MiB 9.44 MiB 1.58 MiB
73dca78 7.86 MiB 9.44 MiB 1.58 MiB
6b69699 7.86 MiB 9.44 MiB 1.58 MiB
f761369 7.86 MiB 9.44 MiB 1.58 MiB
640ad0c 7.86 MiB 9.44 MiB 1.58 MiB
7cfbbd6 7.86 MiB 9.44 MiB 1.58 MiB
e45c0e1 7.86 MiB 9.44 MiB 1.58 MiB
cc4e375 7.86 MiB 9.44 MiB 1.58 MiB

Previous results on branch: enh/ffi-jni-capture-envelope

Startup times

Revision Plain With Sentry Diff
4d265af 1274.31 ms 1284.55 ms 10.24 ms
8243b86 1256.24 ms 1268.31 ms 12.06 ms
5fa2e5f 1233.29 ms 1237.04 ms 3.75 ms
fcdc0a0 1270.39 ms 1282.24 ms 11.86 ms

App size

Revision Plain With Sentry Diff
4d265af 7.86 MiB 9.45 MiB 1.59 MiB
8243b86 7.86 MiB 9.45 MiB 1.59 MiB
5fa2e5f 7.86 MiB 9.45 MiB 1.59 MiB
fcdc0a0 7.86 MiB 9.45 MiB 1.59 MiB

Copy link
Contributor

github-actions bot commented Jul 29, 2025

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 451.48 ms 506.33 ms 54.85 ms
Size 6.54 MiB 7.71 MiB 1.17 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
eca355d 485.50 ms 569.85 ms 84.35 ms
6f47800 451.04 ms 509.64 ms 58.60 ms
2cf9161 454.12 ms 512.67 ms 58.55 ms
cc4e375 426.15 ms 482.34 ms 56.19 ms
793f4dc 462.68 ms 544.21 ms 81.53 ms
b6c8720 457.41 ms 519.04 ms 61.63 ms
3615e19 468.38 ms 504.71 ms 36.33 ms
575ebaa 478.00 ms 585.76 ms 107.76 ms
2d34233 470.54 ms 558.90 ms 88.36 ms
9b99523 456.91 ms 490.55 ms 33.64 ms

App size

Revision Plain With Sentry Diff
eca355d 6.54 MiB 7.70 MiB 1.16 MiB
6f47800 6.54 MiB 7.69 MiB 1.15 MiB
2cf9161 6.54 MiB 7.70 MiB 1.16 MiB
cc4e375 6.54 MiB 7.69 MiB 1.15 MiB
793f4dc 6.54 MiB 7.69 MiB 1.15 MiB
b6c8720 6.54 MiB 7.69 MiB 1.15 MiB
3615e19 6.54 MiB 7.70 MiB 1.16 MiB
575ebaa 6.54 MiB 7.69 MiB 1.15 MiB
2d34233 6.54 MiB 7.55 MiB 1.01 MiB
9b99523 6.54 MiB 7.69 MiB 1.15 MiB

Previous results on branch: enh/ffi-jni-capture-envelope

Startup times

Revision Plain With Sentry Diff
8243b86 480.34 ms 499.35 ms 19.01 ms
5fa2e5f 508.60 ms 556.53 ms 47.93 ms
fcdc0a0 504.15 ms 536.24 ms 32.09 ms

App size

Revision Plain With Sentry Diff
8243b86 6.54 MiB 7.71 MiB 1.17 MiB
5fa2e5f 6.54 MiB 7.71 MiB 1.17 MiB
fcdc0a0 6.54 MiB 7.71 MiB 1.17 MiB

Copy link
Contributor

github-actions bot commented Jul 29, 2025

🚨 Detected changes in high risk code 🚨

High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:

  • flutter/android/src/main/kotlin/io/sentry/flutter/SentryFlutterPlugin.kt

@buenaflor buenaflor marked this pull request as ready for review August 7, 2025 12:08
cursor[bot]

This comment was marked as outdated.

@buenaflor buenaflor requested a review from vaind August 7, 2025 12:39
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@buenaflor buenaflor marked this pull request as draft August 11, 2025 14:10
@buenaflor buenaflor changed the base branch from main to deps/bump-ffigen August 12, 2025 23:48
Comment on lines 60 to 61
// Use a safe copy-based conversion to avoid crashes due to memory issues observed
// when relying on `dataWithBytesNoCopy:length:freeWhenDone:`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you be more specific about what memory issues you've faced?

Copy link
Contributor Author

@buenaflor buenaflor Aug 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vaind it always crashed when using withBytesNoCopy and freeWhenDone set to true (or set to false and manually freeing)

it seemed to be a bug with the current generated ffigen, the new bindings with ffigen 19.0.0 dont have that issue so we can safely use dataWithBytesNoCopy, from manual testing it works fine

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants